Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.09%. Comparing base (858bd1f) to head (f1dcde1).
⚠️ Report is 1 commits behind head on main.

@@           Coverage Diff           @@
##             main     #672   +/-   ##
=======================================
  Coverage   79.09%   79.09%           
=======================================
  Files           9        9           
  Lines        3898     3898           
=======================================
  Hits         3083     3083           
  Misses        815      815           

clang-tidy review says "All clean, LGTM! 👍"

clang-tidy review says "All clean, LGTM! 👍"

clang-tidy review says "All clean, LGTM! 👍"

Current progress with adding compatibility for llvm 21. Can build on all platforms. Can pass all tests for native Windows, native Linux and Emscripten platform. MacOS errors for the following test on both arm and x86

1: [ RUN ] InterpreterTest.IncludePaths
1: LLVM ERROR: Invalid option set!

clang-tidy review says "All clean, LGTM! 👍"

clang-tidy review says "All clean, LGTM! 👍"

For the MacOS failures we apear to be triggering this line

clang-tidy review says "All clean, LGTM! 👍"

Something has happened to the llvm 21 release branch in the last 2 weeks to break the ScopeReflectionTest.GetNumBases test in CppInterOp. Also the Windows build of CppInterOp has broken somehow.

clang-tidy review says "All clean, LGTM! 👍"

Hey @mcbarton ,

Are you working on this ?

I think I saw quite some errors errors while trying to build cppinterop against latest llvm 21 few days back.

clang-tidy review says "All clean, LGTM! 👍"

Hey @mcbarton ,

Are you working on this ?

I think I saw quite some errors errors while trying to build cppinterop against latest llvm 21 few days back.

@anutosh491 With the exception of the Windows native build you can build CppInterOp against llvm 21 with the changes in this PR (Windows fails building llvm). The tests do not pass however. Given the changes in this PR are minimal, then the source of these errors are likely on the llvm side. I will update this PR soon to get rid of the Windows Emscripten cross compile patch since it is no longer needed, and to update the documentation. Not sure I have any spare time at the moment though to work out the source of the failing tests.

clang-tidy review says "All clean, LGTM! 👍"

clang-tidy review says "All clean, LGTM! 👍"

clang-tidy review says "All clean, LGTM! 👍"

clang-tidy review says "All clean, LGTM! 👍"

clang-tidy review says "All clean, LGTM! 👍"

clang-tidy review says "All clean, LGTM! 👍"

@anutosh491 after rebasing this PR yesterday the llvm 21 xeus-cpp builds started to failing with a missing symbol error (see https://github.com/compiler-research/CppInterOp/actions/runs/20308768733/job/58345114330?pr=672#step:14:150) . Before I start debugging, have you seen this symbol before, and know what a fix might be?

Its interesting that the llvm 20 jobs don't require this symbol.

after rebasing this PR yesterday the llvm 21 xeus-cpp builds started to failing with a missing symbol error (see https://github.com/compiler-research/CppInterOp/actions/runs/20308768733/job/58345114330?pr=672#step:14:150) . Before I start debugging, have you seen this symbol before, and know what a fix might be?

Its interesting that the llvm 20 jobs don't require this symbol.

This is an error we expect, if we don't see this we're doing something wrong 😉
Now that we're building wasm-exceptions and not standard emscripten-exceptions, there should be no symbol called emscripten_longjmp when building with wasm exceptions (which is the case throughout emscripten-forge's 4-x branch, so basically all pkgs fetched from the env are built using this and xeus-cpp on main too)

I have expressed my doubt long back (#753 (review)). The PR I linked, makes no change to how llvm or cppinterop was built. There should have been an expected line changing from -fexceptions to -fwasm-exceptions !

after rebasing this PR yesterday the llvm 21 xeus-cpp builds started to failing with a missing symbol error (see https://github.com/compiler-research/CppInterOp/actions/runs/20308768733/job/58345114330?pr=672#step:14:150) . Before I start debugging, have you seen this symbol before, and know what a fix might be?

Its interesting that the llvm 20 jobs don't require this symbol.

This is an error we expect, if we don't see this we're doing something wrong 😉 Now that we're building wasm-exceptions and not standard emscripten-exceptions, there should be no symbol called emscripten_longjmp when building with wasm exceptions (which is the case throughout emscripten-forge's 4-x branch, so basically all pkgs fetched from the env are built using this and xeus-cpp on main too)

I have expressed my doubt long back (#753 (review)). The PR I linked, makes no change to how llvm or cppinterop was built. There should have been an expected line changing from -fexceptions to -fwasm-exceptions !

Thanks i'll take a look into this. I have also managed to work out which commit breaks the macos jobs for CppInterOp. It is the following llvm commit

commit 515b4a4fdd7ac97373b68850a2ffa72e2b8e9178 (HEAD)
Author: Ian Anderson <iana@apple.com>
Date:   Thu May 8 12:30:51 2025 -0700

    [clang][Darwin] Remove legacy framework search path logic in the frontend (#138234)
    
    Move the Darwin framework search path logic from
    InitHeaderSearch::AddDefaultIncludePaths to
    DarwinClang::AddClangSystemIncludeArgs. Add a new -internal-iframework
    cc1 argument to support the tool chain adding these paths.
    Now that the tool chain is adding search paths via cc1 flag, they're
    only added if they exist, so the Preprocessor/cuda-macos-includes.cu
    test is no longer relevant.
    Change Driver/driverkit-path.c and Driver/darwin-subframeworks.c to do
    -### style testing similar to the darwin-header-search and
    darwin-embedded-search-paths tests. Rename darwin-subframeworks.c to
    darwin-framework-search-paths.c and have it test all framework search
    paths, not just SubFrameworks.
    Add a unit test to validate that the myriad of search path flags result
    in the expected search path list.
    
    Fixes https://github.com/llvm/llvm-project/issues/75638

@vgvassilev would you like me to work out a patch for llvm 21 which allows CppInterOp macos jobs to pass, or try and modify CppInterOp to work with this llvm commit which currently breaks the macos jobs? Not sure if the commit description highlights we can work with this change to llvm.

clang-tidy review says "All clean, LGTM! 👍"

would you like me to work out a patch for llvm 21 which allows CppInterOp macos jobs to pass, or try and modify CppInterOp to work with this llvm commit which currently breaks the macos jobs? Not sure if the commit description highlights we can work with this change to llvm.

Wait I'm confused. So it is a cppinterop thing and not a llvm thing ?

As in were we doing something wrong and we need to start maintaining a patch or is that patch at fault and we need to fix it on llvm's side ?

That looks like a 300 line odd patch and not sure anyone of us knows what it does exactly, so do we really want to maintain it ? Let's fix it the organic way if we can, can we ?

would you like me to work out a patch for llvm 21 which allows CppInterOp macos jobs to pass, or try and modify CppInterOp to work with this llvm commit which currently breaks the macos jobs? Not sure if the commit description highlights we can work with this change to llvm.

Wait I'm confused. So it is a cppinterop thing and not a llvm thing ?

As in were we doing something wrong and we need to start maintaining a patch or is that patch at fault and we need to fix it on llvm's side ?

That looks like a 300 line odd patch and not sure anyone of us knows what it does exactly, so do we really want to maintain it ? Let's fix it the organic way if we can, can we ?

That is the question I posed in my comment. The last commit just shows that if we revert that llvm commit I mention above, then the test which errored now passes, showing that llvm commit is responsible for the failure in some way.

Why not post our concerns on that PR and address the root cause ?

Why not post our concerns on that PR and address the root cause ?

We will need to work out why that llvm commit causes CppInterOp macos jobs to fail to tackle the root cause for llvm 22 (since I assume they are unlikely to revert it after months of it being in llvm without anyone raising an issue). At the moment I couldn't post anything in that PR beyond it causes tests to fail for us on MacOS, and don't understand llvm enough to understand the changes in the llvm commit. I found out it was a problematic commit purely through a binary search of llvms commits. For llvm 21 the revert patch will likely be necessary, unless we work out how modify CppInterOp to work with that troublesome commit, since llvm 21s release cycle has come to an end.

Hmm, I am not sure we should maintain random patches which we don't understand or wasn't introduced by one of us :\

I can give it a look, once I back from vacations (Jan 2026)